-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix issue with plotting negative-sum images #562
Conversation
thanks for working on this - it has been on my todo list for several years |
@@ -228,7 +228,7 @@ def reorient_slice(x, axis): | |||
if not isinstance(image, iio.ANTsImage): | |||
raise ValueError("image argument must be an ANTsImage") | |||
|
|||
if not image.sum() > 0: | |||
if sum(sum(image != 0)) == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes CI tests to fail, try
np.all(np.equal(image.numpy(), 0.0))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running viz tests
E
======================================================================
ERROR: test_plot_example (__main__.TestModule_plot)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/runner/work/ANTsPy/ANTsPy/tests/test_viz.py", line 33, in test_plot_example
ants.plot(img, filename=filename)
File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/ants/viz/plot.py", line 231, in plot
if sum(sum(image != 0)) == 0:
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
also - I dont this this should raise an error but instead fail "gracefully" |
ok |
@ncullen93 I can fix this real quick, now it looks like all the other runners are done |
Yes, thanks. Can you consider reducing the number of python versions tested in the CI to 1 or 2 instead of 5? It takes so long otherwise and doesn't add much. I will try to wait for it to finish going forward though. |
Yeah, it can get painful when there's a lot of activity. I will have a think about how we can do better. One option might be to put branch protection on master, and insist that all changes go through PR, and then build a couple of wheels in the PR. That would avoid duplication where PR builds pass and then the same wheels get built again on merge |
If an image has a sum less than zero, then
ants.plot
rejects it. This is due to code that incorrectly identifies whether an image is all-zero.This should work: